Support ArrayTarget as the default and only supported target. #276
Conversation
There was a problem hiding this comment.
Thanks! Looks good to me.
Perhaps we should warn the user when they do not pass -a that the code-based recursive ascent parser has now been removed, hinting that they can get rid of the warning by passing -a (which is a bit strange). In a few versions, we can then remove this warning again. Opinions, @int-index, @andreasabel?
Also be sure to mention #268 in the commit message.
|
I'm surprised to see no changes to the test suite. |
|
Good point; I think we can now get rid of all targets in diff --git a/tests/Makefile b/tests/Makefile
index 84938bd..3e97c35 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -56,42 +56,24 @@ TEST_HAPPY_OPTS = --strict
%.n.hs : %.ly
$(HAPPY) $(TEST_HAPPY_OPTS) $< -o $@
-%.a.hs : %.ly
- $(HAPPY) $(TEST_HAPPY_OPTS) -a $< -o $@
-
%.g.hs : %.ly
$(HAPPY) $(TEST_HAPPY_OPTS) -g $< -o $@
%.gc.hs : %.ly
$(HAPPY) $(TEST_HAPPY_OPTS) -gc $< -o $@
-%.ag.hs : %.ly
- $(HAPPY) $(TEST_HAPPY_OPTS) -ag $< -o $@
-
-%.agc.hs : %.ly
- $(HAPPY) $(TEST_HAPPY_OPTS) -agc $< -o $@
-
%.n.hs : %.y
$(HAPPY) $(TEST_HAPPY_OPTS) $< -o $@
-%.a.hs : %.y
- $(HAPPY) $(TEST_HAPPY_OPTS) -a $< -o $@
-
%.g.hs : %.y
$(HAPPY) $(TEST_HAPPY_OPTS) -g $< -o $@
%.gc.hs : %.y
$(HAPPY) $(TEST_HAPPY_OPTS) -gc $< -o $@
-%.ag.hs : %.y
- $(HAPPY) $(TEST_HAPPY_OPTS) -ag $< -o $@
-
-%.agc.hs : %.y
- $(HAPPY) $(TEST_HAPPY_OPTS) -agc $< -o $@
-
-CLEAN_FILES += *.n.hs *.a.hs *.g.hs *.gc.hs *.ag.hs *.agc.hs *.info *.hi *.bin *.exe *.o *.run.stdout *.run.stderr
+CLEAN_FILES += *.n.hs *.g.hs *.gc.hs *.info *.hi *.bin *.exe *.o *.run.stdout *.run.stderr
-ALL_TEST_HS = $(shell echo $(TESTS) | sed -e 's/\([^\. ]*\)\.\(l\)\{0,1\}y/\1.n.hs \1.a.hs \1.g.hs \1.gc.hs \1.ag.hs \1.agc.hs/g')
+ALL_TEST_HS = $(shell echo $(TESTS) | sed -e 's/\([^\. ]*\)\.\(l\)\{0,1\}y/\1.n.hs \1.g.hs \1.gc.hs/g')
ALL_TESTS = $(patsubst %.hs, %.run, $(ALL_TEST_HS))
@@ -118,15 +100,15 @@ check.%.y : %.y
all :: $(CHECK_ERROR_TESTS) $(ALL_TESTS)
check-todo::
- $(HAPPY) $(TEST_HAPPY_OPTS) -ad Test.ly
+ $(HAPPY) $(TEST_HAPPY_OPTS) -d Test.ly
$(HC) Test.hs -o happy_test
./happy_test
-rm -f ./happy_test
- $(HAPPY) $(TEST_HAPPY_OPTS) -agd Test.ly
+ $(HAPPY) $(TEST_HAPPY_OPTS) -gd Test.ly
$(HC) Test.hs -o happy_test
./happy_test
-rm -f ./happy_test
- $(HAPPY) $(TEST_HAPPY_OPTS) -agcd Test.ly
+ $(HAPPY) $(TEST_HAPPY_OPTS) -gcd Test.ly
$(HC) Test.hs -o happy_test
./happy_test
-rm -f ./happy_test(Some of those targets appear to be duplicated. Weird.) |
Kariiem
force-pushed
the
array-target-only
branch
from
8669956
to
6cbf2b4
Compare
Kariiem
force-pushed
the
array-target-only
branch
from
3b391d5
to
b9257a3
Compare
|
I commited the changes to the |
|
@Ericson2314 @int-index it would be great to make progress here, so that the PR stack that @Kariiem has to wrangle does not become too large. The goal is to merge the changes in #272 bit by bit, starting with the least controversial ones. To me, this PR looks good. It's probably good to run the CI pipeline just in case (which needs maintainer approval.) |
I approved the CI run (again). Is there any way to allow CI to run in all @Kariiem's PRs? @sgraf812 I actually thought you had maintainer access, maybe you could request it? (Admins of the |
| #if defined(HAPPY_GHC) | ||
| happyTcHack :: Happy_GHC_Exts.Int# -> a -> a | ||
| happyTcHack x y = y | ||
| {-# INLINE happyTcHack #-} | ||
| #endif |
There was a problem hiding this comment.
I think these lines should not have been removed; that would explain the CI errors.
Undoing an accidental removal
Alright, I requested and got maintainer access on Matrix! |
|
I merged; the remaining CI errors appear to be unrelated and are hopefully fixed by #279. |
This PR removes all code related to HaskellTarget, makes ArrayTarget on by default independent of the command line arguments and removes extra arguments to several functions that needed to check for the target.
@int-index @Ericson2314 @sgraf812 I hope that a single commit PR is acceptable. I will do the same for
--g/ghcflag andHAPPY_GHC.